Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Add connect wallet button #2328

Merged
merged 6 commits into from
Jan 28, 2022
Merged

Add connect wallet button #2328

merged 6 commits into from
Jan 28, 2022

Conversation

nenadV91
Copy link
Contributor

Summary

Fixes #2297

As defined in the issue:

Adds connect to wallet button on this screen if its not connected
Screenshot from 2022-01-27 13-01-19

Changes the text if its unsupported network
Screenshot from 2022-01-27 13-01-50

@nenadV91 nenadV91 requested review from a team January 27, 2022 12:08
@github-actions
Copy link
Contributor

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@fairlighteth
Copy link
Contributor

Looks good. One comment:

  • I would place the 'or connect a wallet' text button below the input field.

@elena-zh
Copy link

Hey @nenadV91 , great changes!
The same issue as was reported in closed #2309: 'Connect a wallet' button seems like in 'focused' state when navigate to this screen
image

compare:
image

I have just realized that we have another screen where we check available claims for an account: so we show a message about claims in Ethereum, but we are currently connected to Polygon.

image

In order to avoid this, maybe we could disable 'Check' button when connected to an ansupported network? @biocom , @alfetopito , @anxolin , @W3stside , WDYT?
image

@fairlighteth
Copy link
Contributor

fairlighteth commented Jan 27, 2022 •
edited
Loading

In order to avoid this, maybe we could disable 'Check' button when connected to an ansupported network?

I agree this would be good. In addition, we could reflect the same error in the primary button (Wrong network).

@anxolin
Copy link
Contributor

anxolin commented Jan 27, 2022

In order to avoid this, maybe we could disable 'Check' button when connected to an ansupported network? @biocom , @alfetopito , @anxolin , @W3stside , WDYT?

I agree this would be good. In addition, we could reflect the same error in the primary button (Wrong network).

💯

anxolin
anxolin approved these changes Jan 27, 2022
View reviewed changes
Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good ideas from Elena and Michel. We can always address as new PR if we don't want to mix (handle unsupported networks)

src/custom/pages/Claim/ClaimAddress.tsx Outdated Show resolved Hide resolved
@elena-zh
Copy link

Hey @nenadV91 , changes LGTM!

Just as an enhancement, I'd navigate a user to this screen
image
from every claim screen when change a network to an unsupported one.
Currently, it navigates to this screen
image

But anyways, let me know please if I need to create a separate issue for this.

Thanks

@nenadV91
Copy link
Contributor Author

@elena-zh Updated

elena-zh
elena-zh approved these changes Jan 28, 2022
View reviewed changes
Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now!

@nenadV91 nenadV91 merged commit a4a8bd4 into release/1.10 Jan 28, 2022
@alfetopito alfetopito deleted the add-connect-wallet-button branch January 28, 2022 15:32
alfetopito pushed a commit that referenced this pull request Jan 28, 2022
@nenadV91
Add connect wallet button (#2328)
b047e2e 
* Show add to wallet button

* Change message for unsupported network

* PR updates

* Small PR change

* PR update
@fairlighteth fairlighteth mentioned this pull request Feb 1, 2022
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@anxolin anxolin anxolin approved these changes

@elena-zh elena-zh elena-zh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nenadV91 @fairlighteth @elena-zh @anxolin